Skip to content

Conversation

@rnveach
Copy link
Contributor

@rnveach rnveach commented Feb 17, 2025

Description

Removes exceptions not thrown by the methods.

@codecov
Copy link

codecov bot commented Feb 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.57%. Comparing base (5d3344a) to head (15ca8ba).
Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #2036   +/-   ##
=========================================
  Coverage     83.57%   83.57%           
  Complexity     2380     2380           
=========================================
  Files           235      235           
  Lines          7259     7259           
  Branches        382      382           
=========================================
  Hits           6067     6067           
  Misses          956      956           
  Partials        236      236           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bitwiseman
Copy link
Member

Removing exceptions... I thought this was a binary breaking change, but the API checks didn't fail for this change...

I need to look at this further.

@rnveach
Copy link
Contributor Author

rnveach commented Feb 18, 2025

Do you want me to try and split this between main and test?

Also, I would think this would still be acceptable to do to non-public methods in main as users can't write code to link to them outside of reflection anyways.

Finally, can you also provide more information on this project's stance on binary breaking change? Is this project fully against them, or this requires a special release?

Also what checks is used to catch these? I see the POM has japicmp-maven-plugin.

@rnveach
Copy link
Contributor Author

rnveach commented Feb 18, 2025

Documentation:
https://docs.oracle.com/javase/specs/jls/se11/html/jls-13.html#jls-13.4.21 (Java 11 JLS)

Changes to the throws clause of methods or constructors do not break compatibility with pre-existing binaries; these clauses are checked only at compile time.

This says it is not a breaking binary change.

This section also hasn't changed up to JLS 22 ( https://docs.oracle.com/javase/specs/jls/se22/html/jls-13.html#jls-13.4.21 ).

@bitwiseman bitwiseman self-requested a review February 24, 2025 18:17
Copy link
Member

@bitwiseman bitwiseman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating with latest changes. Assuming japicmp reports all clear, we'll take this.

Thanks!

@bitwiseman bitwiseman changed the title removed exceptions not thrown Removed unused checked exceptions Feb 25, 2025
@bitwiseman bitwiseman merged commit 8343b6b into hub4j:main Feb 25, 2025
12 checks passed
@rnveach rnveach deleted the unused_exceptions branch February 26, 2025 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants